fixed duplicate present-proof v2 webhook by sonivijayk · Pull Request #3998 · openwallet-foundation/acapy

sonivijayk · 2025-12-23T22:52:58Z

Fix Summary

Used by_format presence to drop legacy pres_* fields from webhook
Added test for by_format logic based legacy fields exclusion from payload, used dummy payload

Fix Testing

Ran : python -m pytest acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py

jamshale · 2026-01-06T17:13:30Z

I think this is looking good. My only reservation is that it would be considered a breaking change and I'm hesitant to include it in the next release.

I'll review it closer and we can probably get it included in the next, next release.

Also the DCO will need to get fixed.

Copilot

Pull request overview

This PR fixes a webhook duplication issue (BUG #3802) where present-proof v2 webhooks included both legacy pres_* fields and the newer by_format structure, causing duplicate data in webhook payloads.

Key Changes:

Modified webhook emission logic to conditionally exclude legacy fields when by_format is present in debug mode
Extracted and centralized _presentation_request_payload helper function to handle both by_format and legacy payload formats
Added comprehensive test coverage for the webhook payload stripping behavior

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py`	Added conditional logic to strip legacy `pres_*` fields from webhook payloads when `by_format` is present and debug webhooks are enabled
`acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py`	Added test case to verify legacy fields are excluded from webhook payloads when `by_format` exists
`scenarios/examples/util.py`	Added `_presentation_request_payload` helper function and new `indy_present_proof_v2` function; updated `anoncreds_presentation_summary` and `anoncreds_present_proof_v2` to use the helper
`scenarios/examples/simple_restart/example.py`	Updated import to use `indy_present_proof_v2` from util module
`scenarios/examples/multitenancy/example.py`	Updated import to use `indy_present_proof_v2` from util module
`scenarios/examples/presenting_revoked_credential/example.py`	Added local `_presentation_request_payload` function and updated `summary` function to use it; updated import for `indy_present_proof_v2`
`scenarios/examples/kanon_issuance_and_presentation/example.py`	Added local `_presentation_request_payload` function and updated `summary` function to use it
`scenarios/examples/did_indy_issuance_and_revocation/example.py`	Added local `_presentation_request_payload` function and updated `summary` function to use it; updated import for `indy_present_proof_v2`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py

Copilot · 2026-01-06T17:17:38Z

scenarios/examples/presenting_revoked_credential/example.py

+from examples.util import indy_present_proof_v2

 ALICE = getenv("ALICE", "http://alice:3001")
 BOB = getenv("BOB", "http://bob:3001")


+def _presentation_request_payload(presentation: V20PresExRecord):
+    if presentation.by_format and presentation.by_format.pres_request:
+        return presentation.by_format.pres_request
+    request = presentation.pres_request
+    if not request:
+        return None
+    if isinstance(request, dict):
+        return request
+    if hasattr(request, "model_dump"):
+        return request.model_dump(by_alias=True)
+    return request.dict(by_alias=True)
+
+


The function _presentation_request_payload is duplicated in this file. It already exists in scenarios/examples/util.py (lines 63-75) with identical implementation. Consider removing this duplicate and importing it from util.py instead to maintain DRY principles and reduce maintenance burden.

Suggested change

from examples.util import indy_present_proof_v2

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

def _presentation_request_payload(presentation: V20PresExRecord):

if presentation.by_format and presentation.by_format.pres_request:

return presentation.by_format.pres_request

request = presentation.pres_request

if not request:

return None

if isinstance(request, dict):

return request

if hasattr(request, "model_dump"):

return request.model_dump(by_alias=True)

return request.dict(by_alias=True)

from examples.util import indy_present_proof_v2, _presentation_request_payload

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

Copilot · 2026-01-06T17:17:39Z

scenarios/examples/kanon_issuance_and_presentation/example.py

+def _presentation_request_payload(presentation: V20PresExRecord):
+    if presentation.by_format and presentation.by_format.pres_request:
+        return presentation.by_format.pres_request
+    request = presentation.pres_request
+    if not request:
+        return None
+    if isinstance(request, dict):
+        return request
+    if hasattr(request, "model_dump"):
+        return request.model_dump(by_alias=True)
+    return request.dict(by_alias=True)


The function _presentation_request_payload is duplicated in this file. It already exists in scenarios/examples/util.py (lines 63-75) with identical implementation. Consider removing this duplicate and importing it from util.py instead to maintain DRY principles and reduce maintenance burden.

Copilot · 2026-01-06T17:17:39Z

scenarios/examples/did_indy_issuance_and_revocation/example.py

+from examples.util import indy_present_proof_v2

 ALICE = getenv("ALICE", "http://alice:3001")
 BOB = getenv("BOB", "http://bob:3001")


+def _presentation_request_payload(presentation: V20PresExRecord):
+    if presentation.by_format and presentation.by_format.pres_request:
+        return presentation.by_format.pres_request
+    request = presentation.pres_request
+    if not request:
+        return None
+    if isinstance(request, dict):
+        return request
+    if hasattr(request, "model_dump"):
+        return request.model_dump(by_alias=True)
+    return request.dict(by_alias=True)
+
+


The function _presentation_request_payload is duplicated in this file. It already exists in scenarios/examples/util.py (lines 63-75) with identical implementation. Consider removing this duplicate and importing it from util.py instead to maintain DRY principles and reduce maintenance burden.

Suggested change

from examples.util import indy_present_proof_v2

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

def _presentation_request_payload(presentation: V20PresExRecord):

if presentation.by_format and presentation.by_format.pres_request:

return presentation.by_format.pres_request

request = presentation.pres_request

if not request:

return None

if isinstance(request, dict):

return request

if hasattr(request, "model_dump"):

return request.model_dump(by_alias=True)

return request.dict(by_alias=True)

from examples.util import indy_present_proof_v2, _presentation_request_payload

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

Copilot · 2026-01-06T17:17:39Z

scenarios/examples/util.py

 # anoncreds utilities:
+def _presentation_request_payload(
+    presentation: V20PresExRecord,
+) -> Optional[Dict[str, Any]]:


The function _presentation_request_payload is missing a docstring. Add a docstring that explains its purpose, parameters, and return value. For example: "Extract presentation request payload from a presentation exchange record, preferring by_format if available, falling back to legacy pres_request."

Suggested change

) -> Optional[Dict[str, Any]]:

) -> Optional[Dict[str, Any]]:

"""Extract presentation request payload from a presentation exchange record.

This helper prefers the `by_format.pres_request` field when available and

falls back to the legacy `pres_request` field. The legacy value may be a

plain dict or a model instance; in the latter case it is converted to a

dict representation using alias field names.

Args:

presentation: The v2.0 presentation exchange record containing the

formatted and/or legacy presentation request data.

Returns:

A dictionary representation of the presentation request payload, or

None if no request is available on the record.

"""

sonarqubecloud · 2026-01-06T17:32:17Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ff137 · 2026-01-06T18:19:46Z

Re: review request, I'll be able to take a look tomorrow 👌

ff137

Just some minor suggestions before approval

acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py

acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py

scenarios/examples/util.py

ff137 · 2026-01-26T12:25:25Z

@sonivijayk please note this PR will need some finishing touches before it can be merged (mainly DCO sign offs).

sonivijayk · 2026-02-03T05:39:05Z

@sonivijayk please note this PR will need some finishing touches before it can be merged (mainly DCO sign offs).

@ff137 Just double checked. I do not see any DCO sign off pending on the changes committed by me.

ff137 · 2026-02-03T07:13:58Z

@ff137 Just double checked. I do not see any DCO sign off pending on the changes committed by me.

Hmm... there are some merge commits maybe causing an issue. You can try a git rebase to get rid of them:

git fetch origin main
git rebase origin/main  # and resolve potential conflicts
git push --force

That should get the branch up to date and remove extra merge commits.

Edit: I see I can't do a rebase on this branch myself, because there are conflicts (but I can do a merge update). So if the conflicts are hairy (sometimes they really are, when merge commits get rebased), then the simpler way may be to create a temp branch from an up-to-date main, cherry-pick only your 4 commits that you actually want to add, and then overwrite this remote branch from that temp branch. If you want help with the git commands just let me know

swcurran · 2026-02-17T16:31:47Z

@sonivijayk -- are you able to fix the DCO (DCO - Developer Certificate of Origin - https://github.com/apps/dco and https://stackoverflow.com/questions/13043357/git-sign-off-previous-commits on how to sign a previous commit) and investigate/fix the Scenario test failures? Tests are clean in other PRs, so this is likely an issue with the PR itself.

Thanks

sonivijayk · 2026-02-17T16:39:36Z

@sonivijayk -- are you able to fix the DCO (DCO - Developer Certificate of Origin - https://github.com/apps/dco and https://stackoverflow.com/questions/13043357/git-sign-off-previous-commits on how to sign a previous commit) and investigate/fix the Scenario test failures? Tests are clean in other PRs, so this is likely an issue with the PR itself.

Thanks

Hi @swcurran I surely will take a look and fix this tonight.

sonivijayk · 2026-02-18T04:30:57Z

@sonivijayk -- are you able to fix the DCO (DCO - Developer Certificate of Origin - https://github.com/apps/dco and https://stackoverflow.com/questions/13043357/git-sign-off-previous-commits on how to sign a previous commit) and investigate/fix the Scenario test failures? Tests are clean in other PRs, so this is likely an issue with the PR itself.

Thanks

Hi @swcurran

I fixed the sign-off issue on my commit(s).
The remaining DCO failures are on three existing @jamshale commits in this branch history. @jamshale can you please help review and let me know if anything required from me.

regards.

ff137 · 2026-02-18T07:39:15Z

@sonivijayk There are different options for fixing the git history

You can git rebase to try get rid of those additional merge commits:

git fetch origin main
git rebase origin/main  # and resolve potential conflicts
git push --force

They can also be dropped using interactive rebase:

git rebase -i HEAD~10
# add d in front of the merge commits to be dropped

and then rebase on main afterwards.

Alternatively, if you want to avoid resolve conflicts or interactive rebase, then:

make a new branch
cherry pick the commit hash of all the correct commits (there's 4 of them)
force push from the new branch to this branch

# start from clean slate off main branch
git stash
git switch main
git fetch
git pull

# create a temp branch and cherry pick relevant commits
git checkout -b temp  
git cherry-pick 8f6584b521ec459d2060d3f85f4345bead752f3c ff660b63169bfd7f42953c65c2337b75b2c1df04 b82591012c7c87e9558af9ec29b2f571cc77df32 2d6b0f3a26b72ced4aca28333487871e00c7b1dc

Then force push from that temp branch to this one (idk the command off by heart).
That'll override all the other commits

If you need help with any of the git steps, an LLM chat agent should be of assistance

Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com>

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

sonivijayk · 2026-02-19T05:51:31Z

Hi @ff137, I have fixed the commits and test failures. Please take a look.

regards

ff137 · 2026-02-19T13:22:00Z

@sonivijayk Awesome! Glad the DCO could be fixed

Please note my prior review with open comments: #3998 (review)

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

sonivijayk · 2026-02-20T06:52:02Z

@sonivijayk Awesome! Glad the DCO could be fixed

Please note my prior review with open comments: #3998 (review)

Hi @ff137 I have addressed PR comments and ran tests locally and in PR. Please review and let me know.

Thanks

ff137

Great stuff! Thank you for resolving all my requests and nitpicks. I'm happy to approve now

…webhook-dedup

sonarqubecloud · 2026-02-23T15:57:10Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

jamshale · 2026-02-23T17:18:48Z

There's currently a problem with the Interop tests related to the tails server used in the tests.

These tests passed before and the other tests cover this PR so I think we can go ahead and merge.

sonivijayk force-pushed the sonivijayk/fix/issue-3802-presentation-xchg-webhook-dedup branch from c7b95f1 to 33a7849 Compare January 3, 2026 19:46

jamshale requested review from Copilot, esune and ff137 January 6, 2026 17:13

Copilot started reviewing on behalf of jamshale January 6, 2026 17:14 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

ff137 reviewed Jan 7, 2026

View reviewed changes

sonivijayk force-pushed the sonivijayk/fix/issue-3802-presentation-xchg-webhook-dedup branch from a2bf0b2 to 2d6b0f3 Compare February 18, 2026 04:19

sonivijayk force-pushed the sonivijayk/fix/issue-3802-presentation-xchg-webhook-dedup branch from 2d6b0f3 to a8e4966 Compare February 19, 2026 04:11

Vijay Soni added 4 commits February 18, 2026 22:22

fixed duplicate present-proof v2 webhook

c39697e

Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com>

fixed sonar issues

67c1a26

Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com>

fixed linting err

50ec11c

Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com>

fixed test errors

ad718db

Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com>

sonivijayk force-pushed the sonivijayk/fix/issue-3802-presentation-xchg-webhook-dedup branch from a8e4966 to ad718db Compare February 19, 2026 04:22

fixed JSON-LD regression test failure

d3fa5a1

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

sonivijayk force-pushed the sonivijayk/fix/issue-3802-presentation-xchg-webhook-dedup branch from b08b342 to d3fa5a1 Compare February 19, 2026 04:54

Skipping BBS+ flow when not present

07e33ee

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

explicit key presence check in by_format, comment adjusted

3d5e7a7

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

Vijay Kumar Soni added 6 commits February 19, 2026 23:39

added assertion to check 3 keys pres,pres_proposal, pres_request

da84850

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

corrected proof_request selection logic

4255af0

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

corrected proof_request fallback logic

8e7894d

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

indy_present_proof_v2 logic

3ff6357

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

fixed util.py, examples

ac95e4b

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

fixed formatting linting issue

8f66f84

Signed-off-by: Vijay Kumar Soni <vijaysoni@sonivijay.com>

ff137 approved these changes Feb 20, 2026

View reviewed changes

Merge branch 'main' into sonivijayk/fix/issue-3802-presentation-xchg-…

ff0553e

…webhook-dedup

jamshale merged commit 43cca6f into openwallet-foundation:main Feb 23, 2026
12 of 14 checks passed

-) -> Optional[Dict[str, Any]]:
+) -> Optional[Dict[str, Any]]:
+    """Extract presentation request payload from a presentation exchange record.
+    This helper prefers the `by_format.pres_request` field when available and
+    falls back to the legacy `pres_request` field. The legacy value may be a
+    plain dict or a model instance; in the latter case it is converted to a
+    dict representation using alias field names.
+    Args:
+        presentation: The v2.0 presentation exchange record containing the
+            formatted and/or legacy presentation request data.
+    Returns:
+        A dictionary representation of the presentation request payload, or
+        None if no request is available on the record.
+    """

Conversation

sonivijayk commented Dec 23, 2025

Fix Summary

Fix Testing

Uh oh!

jamshale commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 6, 2026

Quality Gate passed

Uh oh!

ff137 commented Jan 6, 2026

Uh oh!

ff137 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ff137 commented Jan 26, 2026

Uh oh!

sonivijayk commented Feb 3, 2026

Uh oh!

ff137 commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swcurran commented Feb 17, 2026

Uh oh!

sonivijayk commented Feb 17, 2026

Uh oh!

sonivijayk commented Feb 18, 2026

Uh oh!

ff137 commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonivijayk commented Feb 19, 2026

Uh oh!

ff137 commented Feb 19, 2026

Uh oh!

sonivijayk commented Feb 20, 2026

Uh oh!

ff137 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 23, 2026

Quality Gate passed

Uh oh!

jamshale commented Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ff137 commented Feb 3, 2026 •

edited

Loading

ff137 commented Feb 18, 2026 •

edited

Loading